Support LT-22605: Add ability to limit HermitCrab parses#452
Support LT-22605: Add ability to limit HermitCrab parses#452jtmaxwell3 wants to merge 5 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #452 +/- ##
==========================================
- Coverage 73.33% 73.29% -0.04%
==========================================
Files 443 443
Lines 37214 37273 +59
Branches 5110 5114 +4
==========================================
+ Hits 27290 27319 +29
- Misses 8801 8829 +28
- Partials 1123 1125 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Hello John, I am still trying to get a grapple on how this all works (getting better). I like the approach - having a deterministic "kill the parse if it takes too long" and it seems like a meaningful improvement over what we have. I investigated it with AI and here are some comments that seem reasonable to me: Looking forward to this landing! |
|
1: I don’t understand why "the feature is broken on the default configuration". It worked fine for me. I’m fine with only bounding the breadth, not the depth. I’m just trying to prevent an exponential explosion, and I don’t think that you can have an exponential depth. |
3 participants
MaxUnapplications was an experimental variable that never got incorporated into FieldWorks. I decided to rename it MaxAlternatives for clarity and to change it to throw a TimeoutException since the MaxUnapplications implementation truncated results without telling the user and since it could produce different results with different parses of the same word because of multi-threading.
MaxAlternatives caps the number of alternatives produced by each stratum during analysis. This is where most of the blowup occurs since rules and templates within a stratum are unordered and since rules and templates are optional during unapplication (analysis). The stratum checks MaxAlternatives as it iterates over the results produced by ApplyTemplates and ApplyMorphological rules. This terminates without a full enumeration because ApplyTemplates and ApplyMorphologicalRules use lazy evaluation (yield return). However, _mrulesRule and _mTemplatesRule do not use lazy evaluation, so we set MaxAlternatives on them as well. Because of this, the maximum number of alternatives can be some multiple of MaxAlternatives in the worst case. We could also pass in the current number of alternatives to _mrulesRule and _mTemplatesRule, but we probably don't need to be that precise. We are just trying to avoid words taking forever to parse.
We could also limit the number of lexical entries found after analysis but the number of lexical entries that are found after analysis is usually proportionate to the number of alternatives, so it probably isn't necessary. Furthermore, we could limit the number of alternatives considered within each stratum during synthesis, but the number of alternatives generated is usually proportionate to the number of lexical entries found because rules are obligatory in the synthesis direction and there are rarely alternative orderings of the rules that match. Limiting the alternatives produced by each stratum should be enough to bound the amount of time spent parsing a word.
NB: I had to wrap the bodies of Parallel.ForEach statements with try ... catch in ParallelCombinationRuleCascade and ParallelPermutationRuleCascade in order to get the TimeoutException to propagate correctly (noted by Devin).
I tested this against Aweti-opap-no-go from Andy Black by setting MaxAlternatives to 1000 and all of the words finished in a reasonable amount of time and the words that were limited gave a reasonable error message.
This change is